refactor: make extended commit info third injected tx #1791
Conversation
| @@ -772,6 +772,10 @@ impl SequencerBlock { | |||
| /// # Panics | |||
| /// | |||
| /// - if a rollup data merkle proof cannot be constructed. | |||
| #[expect( | |||
There was a problem hiding this comment.
Refactor this into a builder.
Needing all the arguments is not a good reason to ignore the lint.
| @@ -1042,6 +1045,16 @@ fn take_initial_elements_from_data( | |||
| `data` used to construct the tree", | |||
| ); | |||
|
|
|||
| let extended_commit_info = if with_extended_commit_info { | |||
There was a problem hiding this comment.
This assigned variable is never used.
There was a problem hiding this comment.
it needs to be taken out of the iter either way, and next PR i will be using this value in SequencerBlock. should i still remove from here?
There was a problem hiding this comment.
I think this is fine to leave as is since this is being merged to a feature branch rather than main.
| @@ -1042,6 +1045,16 @@ fn take_initial_elements_from_data( | |||
| `data` used to construct the tree", | |||
| ); | |||
|
|
|||
| let extended_commit_info = if with_extended_commit_info { | |||
There was a problem hiding this comment.
I think this is fine to leave as is since this is being merged to a feature branch rather than main.
| let res = generate_rollup_datas_commitment(&signed_txs_included, deposits); | ||
|
|
||
| let txs = match encoded_extended_commit_info { | ||
| Some(encoded_extended_commit_info) => { | ||
| std::iter::once(encoded_extended_commit_info.into()) | ||
| .chain(res.into_iter().chain(included_tx_bytes)) | ||
| .collect() | ||
| } | ||
| Some(encoded_extended_commit_info) => res | ||
| .into_iter() | ||
| .chain(std::iter::once(encoded_extended_commit_info.into())) | ||
| .chain(included_tx_bytes) | ||
| .collect(), |
There was a problem hiding this comment.
An Option can be turned into an iterator with 0 or 1 iterations via into_iter, so we can shorten this a little:
// generate commitment to sequence::Actions and deposits and commitment to the rollup IDs
// included in the block, chain on the extended commit info if `Some`, and finally chain on
// the tx bytes.
let txs = generate_rollup_datas_commitment(&signed_txs_included, deposits)
.into_iter()
.chain(encoded_extended_commit_info.map(Bytes::from).into_iter())
.chain(included_tx_bytes)
.collect();
Summary
make extended commit info third injected tx, and update
SequencerBlock::try_from_block_info_and_datato handle whether the extended commit info is in the block or not.Background
it makes more sense to append to the existing list of injected txs which are the two commitments at the start of the block.
Changes
SequencerBlock::try_from_block_info_and_datato handle whether the extended commit info is in the block or notTesting
unit tests
Breaking Changelist
SequencerBlock::try_from_block_info_and_data